Retry user-initiated splices across restarts and disconnects - #930
Retry user-initiated splices across restarts and disconnects#930jkczyz wants to merge 6 commits into
Conversation
|
👋 Thanks for assigning @joostjager as a reviewer! |
3ae2507 to
6098d0e
Compare
joostjager
left a comment
joostjager
left a comment
There was a problem hiding this comment.
The PR description explains that LDK currently does not durably record a splice until signature exchange, but I don’t think it explains why ldk-node should therefore become the owner of that durability?
This seems similar to the functionality added in #882: common enough, and close enough to channel/protocol state, that it feels like it should live in LDK if we want the behavior to be durable. Persisting it in ldk-node means we now have another store tracking protocol state alongside ChannelManager/ChannelMonitor, plus reconciliation logic to infer whether LDK still has or no longer has the splice. That creates desync risk between persistence layers.
I can see ldk-node owning product policy around retries or how/when to surface failures, but the durable record of an accepted splice contribution or in-flight splice intent feels like it should be owned by LDK.
|
This will now need a (likely rather considerable) rebase now that #888 and a few related PRs landed. |
6098d0e to
c005623
Compare
|
I’d still be interested in the rationale for why this needs to live in ldk-node rather than ldk. If ldk drops in-progress splice negotiation state on disconnect or restart before signing, doesn’t every splicing integration need to persist intent, reconcile on restart, retry when appropriate, and surface final failure? |
Hmm... yeah it seems we didn't fully resolve this a couple meetings ago. The conversation was mostly around whether to persist in LDK Node by payment ID or channel ID. Currently, if we have reached quiescence but haven't exchanged signatures, LDK will opportunistically persist an IIUC, even if we persisted an |
|
Right, currently we require the same downstream intent storage for outbound payments and splices (which are often outbound payments). One thing I've thought of is having those methods return a |
c005623 to
7265f8c
Compare
7265f8c to
6e2594c
Compare
6e2594c to
cadfed5
Compare
Retrying a user-initiated splice across restarts requires persisting the splice intent before handing it to LDK, which happens before negotiation and therefore before any funding transaction exists. The pending-payment record was built around an on-chain PaymentDetails carrying a txid, which cannot represent a splice that has not been broadcast yet. Reshape PendingPaymentDetails into an enum: a PendingSplice variant that holds only the generated PaymentId and the splice intent, and a Tracked variant that is the previous record plus an optional intent retained until the splice locks. Add the SpliceIntent and SpliceKind types the intent needs to resubmit or rebuild the contribution. Wallet writes to the pending store go through DataStore::mutate, replacing racy read-then-write pairs. They share one helper whose closure re-reads the payment's status inside the critical section — only Pending payments belong in the pending store, and a status read taken outside it can go stale against graduation — and promotes a bare PendingSplice to a Tracked record once a payment exists under its id: a plain payment-tracking merge would silently no-op against the variant, leaving the splice invisible to txid lookups. This is groundwork; nothing constructs a PendingSplice yet. The classify, retry, and wiring that use it follow in subsequent commits. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A user-initiated splice will be keyed by a PaymentId generated at splice time rather than derived from a candidate's txid, so its retry intent, funding payment, and candidate history all share one record. Teach the classifier to find a pre-broadcast splice intent by its channel and reuse that id, promoting the intent record to a tracked funding payment while preserving the intent until the splice locks. Splices we did not originate (counterparty-initiated or V2 dual-funded opens) keep deriving the id from the first candidate's txid via a fallback. A splice under a generated id is no longer found by the txid-derived lookup, so it leans on find_payment_by_txid's candidate probe to map its txids back to the record. If the intent is already gone when classification runs, the classifier probes those same lookups for a record any candidate already created before minting a txid-derived id, so a wallet sync that recorded the transaction first and a late classification converge on one record. The generic funding classification resolves an existing record the same way before minting a txid-derived id: LDK re-broadcasts a promoted-but-unconfirmed 0conf funding transaction through that path, and the rebroadcast must merge into the record classification already created rather than mint a duplicate. Promotion of a pre-broadcast intent in persist_funding_payment is gated on the payment still being Pending, read inside the pending store's critical section like the rest of the write's decision: a payment that confirmed through ANTI_REORG_DELAY before classification must not re-enter the pending store, which graduation and rebroadcast assume holds only Pending payments. No splice intents are created yet; the splice entry points that persist them follow. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LDK abandons an in-progress splice negotiation whenever the peer disconnects -- which includes stopping the node -- and only durably records a splice once its negotiation reaches signing. A splice dropped before then, after splice_in, splice_out, or bump_channel_funding_fee returned Ok, is therefore silently lost across a restart or an ill-timed disconnect. Persist a splice intent before handing the contribution to LDK, keyed by a PaymentId generated at splice time and reusing the channel's existing intent record when one is present, so a splice and its fee bumps share one id and at most one intent exists per channel. At startup a reconciler probes each intent against LDK's live channel state and resubmits any LDK dropped -- including those lost to a crash before LDK persisted anything -- surfacing SpliceNegotiationFailed only when the channel is gone, a fee bump has nothing left to replace, or the resubmission budget is exhausted. Resubmitting does not require the peer to be connected: LDK holds the contribution and initiates quiescence on reconnect. Dropping an intent must not drop the payment tracking behind it. A crash between classification's two store writes leaves the payment recorded while the pending entry is still pre-broadcast, so the reconciler consults the payment store as well as the entry itself, and clearing the intent promotes such an entry to a tracked funding payment so the payment keeps graduating. A payment no longer Pending already graduated and is not re-indexed. Wallet sync or a restarted broadcast classification can see the splice transaction while only the pre-broadcast intent records it -- the counterparty broadcasts the transaction too, and a crash can leave the intent as the only trace of the splice-time id -- but an intent record carries no txids for the usual lookup to match. Teach both writers to recognize such a transaction by the funding outpoint it spends and adopt the splice-time id, so they converge on one record instead of minting a txid-derived duplicate. A payment-tracking merge (e.g. from wallet sync) must leave a live intent untouched. The splice tests now locate a funding payment by its candidate txid, since its id is generated rather than derived from the funding txid. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A user-initiated splice can fail mid-negotiation while the node is running -- the peer disconnects, or the contribution goes stale behind a competing negotiation -- and LDK reports each such round via SpliceNegotiationFailed. Drive those events through the splice retrier: resubmit the same contribution when the peer merely disconnected, rebuild a fresh one when it went stale, and give up (surfacing the failure) only for a non-retriable reason or once the resubmission budget is exhausted, using LDK's own is_retriable classification. Clear a splice's intent once the channel locks its new funding or the channel closes. Event::SpliceNegotiationFailed is now emitted only when a splice is finally abandoned, not for every failed negotiation round, since a recoverable failure is retried transparently. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add integration coverage for resuming a dropped splice: splice_resumed_after_restart initiates a splice-out while disconnected, restarts the node before anything is negotiated, and asserts the reconciler resumes and completes the splice -- and that a second restart does not resubmit the now-locked splice. splice_rbf_resumed_after_restart does the same for a fee bump. Also cover the id-agreement race: splice_payment_tracked_across_restart_before_lock stops the node right after splice negotiation, lets the counterparty's broadcast confirm while it is down, and asserts after restart that wallet sync and classification -- landing in either order -- produce exactly one payment record, keyed by the splice-time id rather than a txid-derived one, through to Succeeded. Document on splice_in, splice_out, and bump_channel_funding_fee that the splice is retried automatically across restarts until it completes or is given up on. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A splice on a 0conf channel locks before its funding transaction confirms, so LDK promotes the new funding immediately and re-broadcasts the still-unconfirmed transaction on every monitor-update completion, re-typed as a generic funding transaction with wallet-view figures. Exercise the full cycle end to end: the contributing side must keep a single record with the splice-time id, interactive-funding classification, and contribution-derived figures through the re-broadcasts and on to graduation, and the non-contributing side must not record anything. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cadfed5 to
1adcc98
Compare
5 participants
LDK abandons an in-progress splice negotiation whenever the peer disconnects (which includes stopping the node) and only durably records a splice once it reaches signing. Between calling
splice_in/splice_out/bump_channel_funding_feeand that point, a restart or an ill-timed disconnect silently drops the splice with no way to recover it.This makes those splices durable and self-healing: the intent is persisted before the contribution is handed to LDK, and a startup reconciler plus a
SpliceNegotiationFailedhandler resubmit it (gated on LDK'sNegotiationFailureReason::is_retriable) until the splice locks or is genuinely unrecoverable. As a resultSpliceNegotiationFailedis now emitted only once a splice is given up on, rather than for every failed negotiation round.Rather than adding a dedicated store, the intent lives in the existing
PendingPaymentStoreunder aPaymentIdgenerated at splice time — which becomes the splice's payment id, replacing the previous first-candidate-txid derivation. That required modeling the pending record as an enum, since a not-yet-broadcast splice has no funding transaction, and therefore noPaymentDetails, yet. The change is scoped to user-initiated splices; counterparty-initiated splices and V2 opens are untouched and keep the txid-derived id.Restart resumption is covered by two integration tests (a splice-out and an RBF fee-bump), alongside unit coverage of the retry-decision matrix and a test that a promoted 0conf splice's payment record survives LDK's funding re-broadcasts; the existing splice/funding/on-chain suites continue to pass.
Based on #1049.
Generated with assistance from Claude Code (Claude Fable 5).